-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added inbox, actors, external repositories, and conversations. #118
added inbox, actors, external repositories, and conversations. #118
Conversation
seannian
commented
Aug 10, 2023
- added the "Create" function of the Inbox and the skeleton of the others (someone please check if i created the statuses correctly)
- added actors and a method to convert actors into accounts (please check if i created the accounts correctly)
- added external repositories for external actors and statuses
- added the framework for conversations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm super excited to get this in. just a few structural changes.
@@ -45,13 +55,115 @@ public class InboxController { | |||
@Autowired | |||
AccountRepository accountRepository; | |||
|
|||
@Autowired |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from now on we shouldn't wire repositories to controllers. create a service. that way we separate presentation and service logic.
//required to map payload from JSON to a Java Object for data access | ||
ObjectMapper mappedLoad; | ||
|
||
public InboxController(ObjectMapper mappedLoad) { | ||
this.mappedLoad = mappedLoad; | ||
} | ||
|
||
//Print method, testing purposes | ||
public static void printJsonNode(JsonNode node, String indent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should go in the Util class. it is generally useful.
@PostMapping("/inbox") | ||
public Mono<String> inbox(@RequestBody JsonNode inboxNode) { | ||
//print out what it gives | ||
System.out.println("\nSTART####################################################################\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't checkin debug statements unless you use log.debug()
String toLink = objNode.get("id").asText(); | ||
|
||
//putting in variables for now to make it easier to read | ||
String accountName = node.get("actor") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think you use this do you?
|
||
//Making an actor and then converting to account | ||
String accountLink = node.get("actor").asText(); | ||
if (externalActorRepository.findItemById(accountLink) != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will always be true. the mono returned by findItemById will either be empty or have the account, so you should first do a switchIfEmpty to create the account and then you can have common logic for adding the status.
//Making an actor and then converting to account | ||
String accountLink = node.get("actor").asText(); | ||
if (externalActorRepository.findItemById(accountLink) != null) { | ||
externalActorRepository.findItemById(accountLink).flatMap(Actor::convertToAccount).subscribe(account -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do a map rather than subscribe. subscribe will block. then return the mono from the map
externalStatusRepository.save(status); | ||
}); | ||
} | ||
return Mono.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be the mono from the externalStatusRepository.save()
@@ -65,7 +177,7 @@ public Mono<String> followerHandler(String id, JsonNode inboxNode, String reques | |||
String follower = inboxNode.get("actor").asText(); | |||
if (requestType.equals("Follow")) { | |||
// check id | |||
if (accountRepository.findItemByAcct(id)==null) { | |||
if (accountRepository.findItemByAcct(id) == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know this isn't your code, but this is wrong to. it should be no if just:
return accountRepository.findItemByAccount(id).swithIfEmpty(Mono.error(new RuntimeException ....)
.then(followersRepository.findItem ...
@@ -0,0 +1,148 @@ | |||
package edu.sjsu.moth.server.db; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should generate these classes with json2java. the then create a simple class in db with the class as a member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be deleted since we have the generated actor now. right?
…us and ExternalStatusRepository, added services changes to InboxController, new Services, new Util
…tps://github.com/SJSU-CS-systems-group/moth into inbox-actors-external-repositories-conversations
@@ -65,24 +183,24 @@ public Mono<String> followerHandler(String id, JsonNode inboxNode, String reques | |||
String follower = inboxNode.get("actor").asText(); | |||
if (requestType.equals("Follow")) { | |||
// check id | |||
if (accountRepository.findItemByAcct(id)==null) { | |||
if (accountService.getAccount(id) == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't related to the change, but we really need to fix this. this check will always be true, and we aren't connecting the mono to the next one, so we never actually check the account...
@@ -0,0 +1,148 @@ | |||
package edu.sjsu.moth.server.db; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be deleted since we have the generated actor now. right?
public class Conversation { | ||
//https://docs.joinmastodon.org/entities/Conversation/ | ||
public String id; | ||
public boolean unread; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think a boolean is correct here. i think this should be a list of accounts who have read it. we probably want the account ids, not the full account record.
note, this means what we store in the database and the conversation response format will be different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i used a hashmap with <String, Boolean> that maps to <accountId, read>, would that work or should we just have an arraylist of accounts?
//https://docs.joinmastodon.org/entities/Conversation/ | ||
public String id; | ||
public boolean unread; | ||
public List<Account> accounts; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should just use the account ids here too.
public String id; | ||
public boolean unread; | ||
public List<Account> accounts; | ||
public Status lastStatus; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this should also be a status id because we will have the status in the status table right?